test(interop): preserve interop regression-suite fixes + update README#83
Merged
Conversation
Contributor
Greptile SummaryTest-only follow-up that reapplies three interop-harness fixes that missed the #81 merge window: stale
Confidence Score: 5/5Test-only change with no production code touched; safe to merge. All changes are confined to Tests/interop/. The [PY] to [RNS] prefix updates are mechanical and consistent across all four affected files. The new _ensure_rnsd_interface logic is well-guarded and the developer verified a full 14/0 pass run from a clean sim. The SidebandCore teardown swap is a straightforward API alignment. Tests/interop/conftest.py — _lxmd_propagation_hex passes --timeout 10 to lxmd --status; if that flag is not recognised the propagated test silently skips with a misleading warning. Important Files Changed
Sequence DiagramsequenceDiagram
participant pytest
participant sim as sim fixture
participant esi as _ensure_sim_interface
participant bp as _bootstrap_paths
participant sb as sideband fixture
participant lxmd
participant Columba as Columba.app (sim)
participant Sideband as SidebandPeer
pytest->>sim: create Simulator(udid)
sim-->>pytest: Simulator instance
pytest->>sb: SidebandPeer.start()
sb-->>pytest: peer ready
pytest->>esi: _ensure_rnsd_interface(udid)
esi->>Columba: simctl terminate
esi->>esi: write com.columba.interfaces plist
esi->>Columba: kickstart cfprefsd
esi->>Columba: simctl launch
esi->>Columba: poll diag.log for [RNS] wrote config
Columba-->>esi: N interfaces registered
pytest->>bp: _bootstrap_paths(sim, sideband, _ensure_sim_interface)
bp->>Sideband: lxmf_destination.announce()
bp->>Columba: lxma://test-announce
loop until both paths known
bp->>Columba: poll diag.log for Sideband announce
bp->>Sideband: RNS.Identity.recall(ios_hex)
end
bp->>lxmd: lxmd --status --timeout 10
lxmd-->>bp: Propagation Node running on hash
bp->>sim: inject _cached_propagation_node_hex
Note over pytest,Sideband: 14 interop tests run
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
Tests/interop/conftest.py:203
**`--timeout` may not be a recognised `lxmd --status` flag**
`_sh([LXMD_BIN, "--status", "--timeout", "10"], check=False, timeout=20)` passes `--timeout` as a positional flag to `lxmd`. If the installed version of lxmd doesn't recognise `--timeout` for the `--status` subcommand, it exits non-zero immediately, `r.stdout` is empty, the regex finds nothing, and `_lxmd_propagation_hex` returns `None` — meaning `test_image_..._propagated` silently skips with a "lxmd down?" warning even though lxmd is perfectly healthy. The subprocess-level `timeout=20` and the `except Exception: return None` guard handle binary-not-found and hangs correctly, but there's no stderr check to distinguish "flag unrecognised" from "lxmd truly unreachable". Dropping `--timeout 10` (or falling back without it on `CalledProcessError`) would remove the dependency on an undocumented CLI contract.
Reviews (2): Last reviewed commit: "docs(interop): update README for the aut..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| # start (see _ensure_rnsd_interface), so a fresh install / clean sim needs no | ||
| # manual Settings → Network Interfaces → Add step. Override for a non-default host. | ||
| RNSD_TCP_HOST = os.environ.get("RNSD_TCP_HOST", "127.0.0.1") | ||
| RNSD_TCP_PORT = int(os.environ.get("RNSD_TCP_PORT", "4242")) |
Contributor
There was a problem hiding this comment.
RNSD_TCP_PORT is set to a non-integer value (e.g. accidentally including whitespace, an alphabetic character, or an empty string), int() raises a bare ValueError at module import time — before pytest collects any tests and before any fixture can emit a useful diagnostic. A try/except with a clear message gives a much better failure experience.
Suggested change
| RNSD_TCP_PORT = int(os.environ.get("RNSD_TCP_PORT", "4242")) | |
| try: | |
| RNSD_TCP_PORT = int(os.environ.get("RNSD_TCP_PORT", "4242")) | |
| except ValueError: | |
| raise SystemExit( | |
| f"RNSD_TCP_PORT={os.environ['RNSD_TCP_PORT']!r} is not a valid integer port number." | |
| ) |
Prompt To Fix With AI
This is a comment left during a code review.
Path: Tests/interop/conftest.py
Line: 60
Comment:
If `RNSD_TCP_PORT` is set to a non-integer value (e.g. accidentally including whitespace, an alphabetic character, or an empty string), `int()` raises a bare `ValueError` at module import time — before pytest collects any tests and before any fixture can emit a useful diagnostic. A `try/except` with a clear message gives a much better failure experience.
```suggestion
try:
RNSD_TCP_PORT = int(os.environ.get("RNSD_TCP_PORT", "4242"))
except ValueError:
raise SystemExit(
f"RNSD_TCP_PORT={os.environ['RNSD_TCP_PORT']!r} is not a valid integer port number."
)
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+155
to
+181
| targets = [_app_data_container(udid) / "Library/Preferences" / f"{BUNDLE_ID}.plist"] | ||
| ag = _app_group_prefs_plist(udid) | ||
| if ag is not None: | ||
| targets.append(ag) | ||
| for plist_path in targets: | ||
| try: | ||
| with open(plist_path, "rb") as f: | ||
| pl = plistlib.load(f) | ||
| except Exception: | ||
| pl = {} | ||
| pl["com.columba.interfaces"] = blob # bytes -> <data>, exactly UserDefaults.set(Data) | ||
| plist_path.parent.mkdir(parents=True, exist_ok=True) | ||
| with open(plist_path, "wb") as f: | ||
| plistlib.dump(pl, f, fmt=plistlib.FMT_BINARY) | ||
|
|
||
| # cfprefsd caches container prefs; kick it AFTER editing so it re-reads our | ||
| # file rather than serving / flushing back the stale in-memory copy | ||
| # (mirrors clean_location_state's dance). user/501 is the sim's only scope. | ||
| subprocess.run(["xcrun", "simctl", "spawn", udid, "launchctl", "kickstart", "-k", | ||
| "user/501/com.apple.cfprefsd.xpc.daemon"], capture_output=True) | ||
| time.sleep(0.5) | ||
| subprocess.run(["xcrun", "simctl", "launch", udid, BUNDLE_ID], check=True, capture_output=True) | ||
|
|
||
| # Confirm the backend loaded the interface (config written with N>=1), so a | ||
| # silent seed failure surfaces here rather than as a confusing path-bootstrap | ||
| # timeout. diag.log is cleared on launch, so the match is fresh. | ||
| diag = _app_data_container(udid) / "Documents" / "diag.log" |
Contributor
There was a problem hiding this comment.
_app_data_container called twice per seed
_ensure_rnsd_interface shells out to xcrun simctl get_app_container at line 155 (building targets) and again at line 181 (resolving diag). Each call forks a subprocess. Capture the result once and reuse it — container = _app_data_container(udid) — to halve the subprocess overhead and keep the path consistent across both uses.
Prompt To Fix With AI
This is a comment left during a code review.
Path: Tests/interop/conftest.py
Line: 155-181
Comment:
**`_app_data_container` called twice per seed**
`_ensure_rnsd_interface` shells out to `xcrun simctl get_app_container` at line 155 (building `targets`) and again at line 181 (resolving `diag`). Each call forks a subprocess. Capture the result once and reuse it — `container = _app_data_container(udid)` — to halve the subprocess overhead and keep the path consistent across both uses.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
The dual-backend refactor renamed the diag.log backend prefix from the Python-specific [PY] to backend-agnostic [RNS] (AppServices logs [RNS] started/inbound/announce/delivery), but the interop harness still grepped [PY] — so identity_hex/lxmf_delivery_hex/auto_propagation_node_hex and the inbound assertion never matched and the whole suite errored at setup (_bootstrap_paths). Update conftest regexes (incl. the escaped re.search patterns), the test_attachments inbound assert, the readiness poll, run_peer, and comments. No production change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…d suite)
The suite assumed the sim's Columba was pre-configured with a TCP-client
interface to the host rnsd/lxmd shared instance; a fresh install came up
with 0 interfaces, so _bootstrap_paths timed out ('sim never heard the
peer announce'). Add a session-scoped autouse fixture that seeds exactly
that interface into Columba's UserDefaults interface store and relaunches,
so the regression suite needs no manual 'Add Interface' step.
InterfaceRepository persists interfaces as a JSON blob under
com.columba.interfaces (app-group suite, migrated from .standard). We write
the blob directly to the container plist(s) — simctl spawn defaults targets
the wrong (global) domain — and bounce cfprefsd so the relaunched app
re-reads it. Host/port override via RNSD_TCP_HOST/RNSD_TCP_PORT. Verified:
from a wiped-interface sim, the fixture seeds + the suite passes.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… skip) + fix Sideband teardown
- test_image_..._propagated skipped whenever the sim hadn't heard lxmd's
5-min lxmf.propagation announce yet (ordering flake). lxmd has no announce
trigger and RNS.Transport.request_path is a no-op from a shared-instance
process that already holds the path; a real re-announce needs an lxmd
restart (drops the whole mesh — wrong for a regression run). Instead
discover lxmd's prop-node hash via
LXMF Propagation Node running on <33f2621f135146ce30f0767d811af2b6>, uptime is 33m and 18.48s
Messagestore contains 3265 messages, 8.25 MB (0.41% utilised of 2.00 GB)
Required propagation stamp cost is 16, flexibility is 3
Peering cost is 18, max remote peering cost is 26
Accepting propagated messages from all nodes
256.00 KB message limit, 10.24 MB sync limit
Peers : 17 total (peer limit is 20)
17 discovered, 0 static
17 available, 0 unreachable
Traffic : 8 messages received in total (2.43 KB)
0 messages received from peered nodes (0 B)
8 messages received from unpeered nodes (2.43 KB)
0 messages transferred to peered nodes (0 B)
0 propagation messages received directly from clients
8 propagation messages served to clients
Distribution factor is 0.0 in _bootstrap_paths
(after sim+sideband connect) and pin it on the sim helper, so the test runs
deterministically and still exercises the real propagated delivery path
(sim -> lxmd store -> Sideband sync). Falls back to a warn+skip if lxmd is
down. -> full suite now 14 passed, 0 skipped.
- peer_sideband.stop(): SidebandCore renamed teardown to cleanup() (the old
__exit_handler is gone); call cleanup() instead so we don't leak the
hasattr-guard warning. (Caught by the guard added last commit.)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Bring the harness README in line with the working suite: lxmd (not rnsd) owns the shared RNS instance + :4242 TCPServer (rnsd/nomadnet are clients); the suite auto-seeds the sim's TCP interface and pins lxmd's propagation node, so no manual interface setup; diag prefix is [RNS]; `pytest -v` is the run command (14 cases). Add a troubleshooting section incl. the wedged-lxmd symptom + `launchctl kickstart` fix. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
torlando-tech
force-pushed
the
fix/interop-regression-suite
branch
from
88db1b7 to
c720607
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Test-only follow-up. These interop-harness fixes were developed while validating the dual-backend stack on-device but landed after #81 was already merged, so they didn't make it into
main(the suite there still has the old[PY]diag prefix and would error at setup). Reapplied here ontomain. No production code — onlyTests/interop/.What's included:
[RNS]diag-log prefix (was[PY]). The dual-backend refactor renamed the backend-agnostic log prefix; the harness regexes/asserts were stale and errored the whole suite at setup.conftest._ensure_rnsd_interface). A fresh sim comes up with 0 RNS interfaces; the fixture now seeds127.0.0.1:4242into Columba's interface store + relaunches, so the suite is self-contained (no manual "Add Interface"). Override viaRNSD_TCP_HOST/RNSD_TCP_PORT.test_image_..._propagatedruns deterministically instead of skipping on lxmd's 5-min announce cadence; fix Sideband teardown (_exit_handler→cleanup(), removed upstream).:4242TCPServer), documented the automated run (pytest -v, 14 cases), and added a troubleshooting section (incl. the wedged-lxmd symptom +launchctl kickstartfix).Verified: from a wiped-interface sim, the fixtures seed everything and the full suite is 14 passed, 0 skipped.